Unify view create/edit via right-side Panel, replace dual implementation - #688
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ype-specific options - Add UnifiedViewConfig type to @object-ui/types - Add mode="create"|"edit" prop to ViewConfigPanel - Add type-specific options (kanban/calendar/map/gallery/timeline/gantt) - Add onCreate callback for create mode - Update ObjectView: "Add View" opens panel in create mode - Keep ViewDesigner as "Advanced Editor" entry - Add i18n keys for new features - Add 14 new tests (49 total passing) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
- Update "Add View" tests to verify panel opens instead of navigation - Add test for "Advanced Editor" button navigation - Update ROADMAP.md with completed P1.8 items Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR unifies the view creation and editing experience by consolidating two separate implementations (full-screen ViewDesigner vs. sidebar ViewConfigPanel) into a single panel-based flow. The changes align with Airtable-style UX patterns and maintain backward compatibility while significantly improving code maintainability.
Changes:
- Introduced
UnifiedViewConfigtype in@object-ui/typesthat combines flat view properties with nested type-specific options for kanban, calendar, map, gallery, timeline, gantt, and chart views - Extended
ViewConfigPanelwithmode="create"|"edit"prop and type-specific configuration sections, allowing it to handle both view creation and editing flows - Rewired
ObjectViewentry points so "Add View" opens the config panel instead of navigating to ViewDesigner, while retaining ViewDesigner as "Advanced Editor"
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/index.ts | Exports new UnifiedViewType and UnifiedViewConfig types |
| packages/types/src/designer.ts | Defines UnifiedViewConfig interface with type-specific nested options per @objectstack/spec protocol |
| packages/i18n/src/locales/en.ts | Adds English translations for create mode UI and type-specific field labels |
| apps/console/src/components/ViewConfigPanel.tsx | Implements mode prop, onCreate callback, and conditional type-specific options rendering |
| apps/console/src/components/ObjectView.tsx | Replaces navigation-based "Add View" with panel opening; adds "Advanced Editor" menu item |
| apps/console/src/tests/ViewConfigPanel.test.tsx | Adds 14 new tests covering create mode and type-specific options (49 total) |
| apps/console/src/tests/ObjectView.test.tsx | Updates 2 tests for panel flow, adds 1 test for Advanced Editor navigation |
| ROADMAP.md | Marks P1.8 items as completed (unified create/edit, type-specific options, etc.) |
| useEffect(() => { | ||
| setDraft({ ...activeView }); | ||
| setIsDirty(false); | ||
| }, [activeView.id]); | ||
| setDraft({ ...effectiveActiveView }); | ||
| setIsDirty(mode === 'create'); | ||
| }, [mode, activeView.id]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
The useEffect has an eslint-disable comment for exhaustive-deps, but is missing effectiveActiveView in the dependency array. Since effectiveActiveView is computed from defaultNewView (which depends on mode being 'create'), changes to mode should trigger re-evaluation, but the effect won't pick up changes to the memoized defaultNewView content itself. Consider either adding effectiveActiveView to the deps or ensuring the memo dependencies are correct.
| createView: 'Create View', | ||
| newView: 'New View', | ||
| advancedEditor: 'Advanced Editor', | ||
| typeOptions: 'Type Options', | ||
| groupByField: 'Group by field', | ||
| startDateField: 'Start date field', | ||
| titleField: 'Title field', | ||
| latitudeField: 'Latitude field', | ||
| longitudeField: 'Longitude field', | ||
| imageField: 'Image field', | ||
| dateField: 'Date field', | ||
| selectField: 'Select field...', | ||
| gridOptionsHint: 'Grid view uses the columns configured above.', |
There was a problem hiding this comment.
The new i18n keys added to the English locale (createView, newView, advancedEditor, typeOptions, groupByField, startDateField, titleField, latitudeField, longitudeField, imageField, dateField, selectField, gridOptionsHint) are missing from other locale files (zh, ar, de, es, fr, ja, ko, pt, ru). This will cause untranslated keys to appear in non-English locales.
|
|
||
| // Default empty view for create mode | ||
| const defaultNewView = useMemo(() => ({ | ||
| id: `view_${Date.now()}`, |
There was a problem hiding this comment.
Using Date.now() for ID generation can lead to collisions if multiple views are created in rapid succession. Consider using crypto.randomUUID() instead, which is already used elsewhere in the codebase for generating unique IDs (as seen in the filter/sort builder code).
| id: `view_${Date.now()}`, | |
| id: `view_${crypto.randomUUID()}`, |
| const handleViewCreate = useCallback(async (config: Record<string, any>) => { | ||
| try { | ||
| if (dataSource?.create) { | ||
| const payload = { objectName, ...config }; | ||
| await dataSource.create('sys_view', payload); | ||
| } | ||
| setShowViewConfigPanel(false); | ||
| setViewConfigPanelMode('edit'); | ||
| setRefreshKey(k => k + 1); | ||
| } catch (err) { | ||
| console.error('[ViewConfigPanel] Failed to create view:', err); | ||
| } | ||
| }, [dataSource, objectName]); |
There was a problem hiding this comment.
The handleViewCreate function doesn't provide user feedback upon successful view creation or when an error occurs. Consider adding toast notifications (success/error) to inform users about the operation outcome, similar to the pattern used in ViewDesignerPage.
| {viewType !== 'grid' && ( | ||
| <> | ||
| <SectionHeader title={t('console.objectView.typeOptions')} /> | ||
| <div data-testid="type-options-section" className="space-y-2"> | ||
| {viewType === 'kanban' && ( | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.groupByField')}</span> | ||
| <select | ||
| data-testid="type-opt-kanban-groupByField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.kanban?.groupByField || draft.kanban?.groupField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('kanban', 'groupByField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {viewType === 'calendar' && ( | ||
| <> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.startDateField')}</span> | ||
| <select | ||
| data-testid="type-opt-calendar-startDateField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.calendar?.startDateField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('calendar', 'startDateField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.titleField')}</span> | ||
| <select | ||
| data-testid="type-opt-calendar-titleField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.calendar?.titleField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('calendar', 'titleField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| </> | ||
| )} | ||
| {viewType === 'map' && ( | ||
| <> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.latitudeField')}</span> | ||
| <select | ||
| data-testid="type-opt-map-latitudeField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.map?.latitudeField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('map', 'latitudeField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.longitudeField')}</span> | ||
| <select | ||
| data-testid="type-opt-map-longitudeField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.map?.longitudeField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('map', 'longitudeField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| </> | ||
| )} | ||
| {viewType === 'gallery' && ( | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.imageField')}</span> | ||
| <select | ||
| data-testid="type-opt-gallery-imageField" | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft.gallery?.imageField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange('gallery', 'imageField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {(viewType === 'timeline' || viewType === 'gantt') && ( | ||
| <> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.dateField')}</span> | ||
| <select | ||
| data-testid={`type-opt-${viewType}-dateField`} | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft[viewType]?.dateField || draft[viewType]?.startDateField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange(viewType, 'dateField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <span className="text-xs text-muted-foreground">{t('console.objectView.titleField')}</span> | ||
| <select | ||
| data-testid={`type-opt-${viewType}-titleField`} | ||
| className="w-full text-xs h-7 rounded-md border border-input bg-background px-2 text-foreground mt-1" | ||
| value={draft[viewType]?.titleField || ''} | ||
| onChange={(e: React.ChangeEvent<HTMLSelectElement>) => handleTypeOptionChange(viewType, 'titleField', e.target.value)} | ||
| > | ||
| <option value="">{t('console.objectView.selectField')}</option> | ||
| {fieldOptions.map(f => ( | ||
| <option key={f.value} value={f.value}>{f.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| </> | ||
| )} | ||
| </div> | ||
| </> |
There was a problem hiding this comment.
The "chart" view type is included in VIEW_TYPE_OPTIONS and can be selected in the view type dropdown, but there is no corresponding type-specific options UI section for chart configuration (unlike kanban, calendar, map, gallery, timeline, gantt). This means users can select "chart" as a view type but cannot configure any chart-specific options (chartType, xAxisField, yAxisFields, etc.) through the panel. Consider either: (1) removing "chart" from VIEW_TYPE_OPTIONS if it's not yet supported in the panel UI, or (2) adding a basic chart configuration section, or (3) adding a comment explaining that chart configuration is intentionally omitted due to complexity.
| showSearch: true, | ||
| showFilters: true, | ||
| showSort: true, | ||
| }), []); |
There was a problem hiding this comment.
The useMemo hook for defaultNewView is missing the t function in its dependency array. The t function from useObjectTranslation() is used inside the memoization but not listed as a dependency, which could cause stale closure issues if the translation function changes.
| }), []); | |
| }), [t]); |
New and edit view flows used two separate implementations (full-screen ViewDesigner vs. sidebar ViewConfigPanel) with overlapping features but different data models and UX. This unifies them into a single panel-based flow.
UnifiedViewConfigtype (@object-ui/types)ViewConfigPanel —
modeprop + type-specific optionsmode="create" | "edit"(default"edit") — create mode initializes empty draft withisDirty=trueonCreatecallback separate fromonSavefor create flowObjectView — rewired entry points
mode="create"instead of navigating to ViewDesignerviewConfigPanelModestate andhandleViewCreatecallbacki18n + ROADMAP
createView,newView,advancedEditor,typeOptions, field labelsTests
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.